Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix 'TypeError: this is undefined' #860

Closed
wants to merge 1 commit into from
Closed

Fix 'TypeError: this is undefined' #860

wants to merge 1 commit into from

Conversation

danielattilasimon
Copy link

@danielattilasimon danielattilasimon commented May 31, 2020 •
edited
Loading

Because populateCallTransaction is not called as a method, this line of code was throwing TypeError: this is undefined at runtime for me in the browser. I still don't understand why my tests are passing though...

Any ideas? Is this a valid fix?

UPDATE: hmm, could be that during tests the from override is never set.

@ricmoo ricmoo added bug Verified to be an issue. on-deck This Enhancement or Bug is currently being worked on. labels May 31, 2020
@ricmoo
Copy link
Member

ricmoo commented Jun 1, 2020

(I'm adding testing to further make sure everything else works too)

ricmoo added a commit that referenced this pull request Jun 1, 2020
@ricmoo
Major Contract refactor for overrides (#819, #845, #847, #860).
42dee67
@ricmoo
Copy link
Member

ricmoo commented Jun 1, 2020

This should be fixed in 5.0.0-beta.190.

There was quite. bit of refactoring, but there are now more tests in place to catch these. I still need to add a lot more tests, but for now this should cover the obvious things.

Let me know! :)

@danielattilasimon
Copy link
Author

Awesome, this seems to be fixed, thanks!

One thing I noticed in the new version is, if I pass { blockTag: undefined } as overrides to a call now, it throws:

Error: cannot override "blockTag" (operation="overrides", overrides=["blockTag"], code=UNSUPPORTED_OPERATION, version=contracts/5.0.0-beta.155)

I will change my code so it only includes blockTag in overrides if it's not undefined, but is this intended behavior?

Thanks again for the quick fix!

@ricmoo
Copy link
Member

ricmoo commented Jun 1, 2020

Eek. I’ll add a test case and a fix for that in the morning. :)

Thanks for all the testing. :)

@danielattilasimon
Copy link
Author

Cool, thanks so much!

No worries, I love using Ethers 5 :)

ricmoo added a commit that referenced this pull request Jun 3, 2020
@ricmoo
Allow undefined properties in transaction object and fix stray this (#…
98bb589 
…860).
ricmoo added a commit that referenced this pull request Jun 3, 2020
@ricmoo
Added test case for null from and blockTag to contract populateTransa…
d2406c4 
…ction (#860).
@ricmoo
Copy link
Member

ricmoo commented Jun 3, 2020

This should be fixed in 5.0.0-beta.191. Try it out and let me know. :)

@danielattilasimon
Copy link
Author

Indeed it's fixed! Thanks so much! :)

michaeltout pushed a commit to michaeltout/ethers.js that referenced this pull request Aug 23, 2020
@ricmoo
Major Contract refactor for overrides (ethers-io#819, ethers-io#845, e…
39e9c07 
…thers-io#847, ethers-io#860).
michaeltout pushed a commit to michaeltout/ethers.js that referenced this pull request Aug 23, 2020
@ricmoo
Major Contract refactor for overrides (ethers-io#819, ethers-io#845, e…
0018446 
…thers-io#847, ethers-io#860).
@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danielattilasimon @ricmoo